Implement UseUserAccessGroup for Firebase Auth C++ SDK#1757
Closed
jonsimantov wants to merge 3 commits intomainfrom
Closed
Implement UseUserAccessGroup for Firebase Auth C++ SDK#1757jonsimantov wants to merge 3 commits intomainfrom
jonsimantov wants to merge 3 commits intomainfrom
Conversation
This commit adds the UseUserAccessGroup method to the Firebase Auth C++ SDK. - On iOS, this method calls the native [FIRAuth useUserAccessGroup:error:] method to specify a user access group for iCloud keychain access. - On other platforms (desktop, Android), this method is a no-op and returns kAuthErrorNone as it's an iOS-only feature. Public API has been added to firebase::auth::Auth in auth.h, with implementations in auth_ios.mm (iOS) and auth_desktop.cc (stub for other platforms).
jonsimantov
commented
Jun 27, 2025
| auth_impl->assigned_emulator_url.append(std::to_string(port)); | ||
| } | ||
|
|
||
| AuthError Auth::UseUserAccessGroup(const char* access_group) { |
Contributor
Author
There was a problem hiding this comment.
We need an Android stub as well.
jonsimantov
commented
Jun 27, 2025
| /// This method is only functional on iOS. On other platforms, it is a no-op | ||
| /// and will always return `kAuthErrorNone`. | ||
| /// | ||
| /// If you are using iCloud keychain synchronization, you will need to call |
Contributor
Author
There was a problem hiding this comment.
Remove this sentence.
jonsimantov
commented
Jun 27, 2025
| /// this method to set the user access group. | ||
| /// | ||
| /// @param[in] access_group The user access group to use. Set to `nullptr` or | ||
| /// an empty string to use the default access group. |
Contributor
Author
There was a problem hiding this comment.
Let's pass in an empty string verbatim instead of treating it as nil.
jonsimantov
commented
Jun 27, 2025
Contributor
Author
jonsimantov
left a comment
jonsimantov
left a comment
There was a problem hiding this comment.
Could you add a VERY short test to the Analytics integration test that just runs this method to confirm it runs?
jonsimantov
commented
Jun 27, 2025
|
|
||
| AuthError Auth::UseUserAccessGroup(const char* access_group_str) { | ||
| if (!auth_data_) { | ||
| return kAuthErrorUninitialized; |
Contributor
Author
There was a problem hiding this comment.
No such error, this one doesn't exist, please check the AuthError enum and suggest a better option.
- Added Android stub for UseUserAccessGroup. - Updated Doxygen comments in auth.h for clarity and accuracy regarding empty string handling. - Modified iOS implementation to pass empty strings verbatim to the native SDK. - Corrected error code used in iOS implementation when auth_data_ is null (kAuthErrorFailure instead of kAuthErrorUninitialized). - Added a basic smoke test to integration_test.cc for iOS to ensure UseUserAccessGroup can be called.
jonsimantov
commented
Jun 27, 2025
The test now calls UseUserAccessGroup without checking its return value. This is to prevent failures in environments where keychain sharing might not be configured, as the primary goal of this test is to ensure the method call itself doesn't crash on iOS.
jonsimantov
commented
Jun 27, 2025
|
|
||
| #endif // defined(ENABLE_OAUTH_TESTS) | ||
|
|
||
| #if TARGET_OS_IPHONE |
Contributor
Author
There was a problem hiding this comment.
Since we have this stubbed out on other platforms, let's make sure that works and run this test on all platforms.
jonsimantov
commented
Jun 27, 2025
| /// | ||
| /// @param[in] access_group The user access group to use. Set to `nullptr` | ||
| /// to use the default access group. An empty string will be passed as an | ||
| /// empty string. |
Contributor
Author
There was a problem hiding this comment.
We don't need to call out that the empty string is passed in special.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit adds the UseUserAccessGroup method to the Firebase Auth C++ SDK.
Public API has been added to firebase::auth::Auth in auth.h, with implementations in auth_ios.mm (iOS) and auth_desktop.cc (stub for other platforms).
Description
Testing
Type of Change
Place an
xthe applicable box:Notes
Release Notessection ofrelease_build_files/readme.md.